Skip to content

Conversation

@jordan-umusu
Copy link
Collaborator

@jordan-umusu jordan-umusu commented Jan 29, 2026

Summary by cubic

Seeds RBAC defaults at startup and during registry sync: system scopes and preset org roles, plus registry action scopes. Removes role fields from memberships/invitations and updates invitations to use role_id/role_slug.

  • New Features
    • Seed system scopes and preset roles for all orgs at startup (and on org creation) via seed_all_system_data.
    • Seed execute scopes for synced registry actions (bulk, idempotent).
    • Non-blocking seeding with logging; startup and sync continue on errors.

Written for commit 1246c82. Summary will update on new commits.

@jordan-umusu
Copy link
Collaborator Author

@cubic review

@jordan-umusu
Copy link
Collaborator Author

@codex review

@cubic-dev-ai
Copy link
Contributor

cubic-dev-ai bot commented Feb 2, 2026

@cubic review

@jordan-umusu I have started the AI code review. It will take a few minutes to complete.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b43f87dcc5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 issues found across 22 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tracecat/organization/schemas.py">

<violation number="1" location="tracecat/organization/schemas.py:46">
P1: Missing validation: the docstring states that either `role_id` or `role_slug` must be provided, but both fields default to `None` with no enforcement. Add a Pydantic `model_validator` to ensure at least one is provided.

(Based on your team's feedback about using Pydantic v2 model_validator for cross-field constraints.) [FEEDBACK_USED]</violation>
</file>

<file name="tracecat/registry/sync/jobs.py">

<violation number="1" location="tracecat/registry/sync/jobs.py:199">
P2: Avoid broad `except Exception` in this business-logic helper. Catch only expected DB/validation exceptions (e.g., SQLAlchemy `IntegrityError`) and let unexpected errors bubble to the caller or a higher-level handler so they are not silently swallowed.

(Based on your team's feedback about error handling posture.) [FEEDBACK_USED]</violation>
</file>

<file name="tracecat/authz/seeding.py">

<violation number="1" location="tracecat/authz/seeding.py:363">
P3: Use tuples() for the multi-column select result to follow the project’s SQLAlchemy result handling guidance.

(Based on your team's feedback about using result.tuples() for multi-column selects.) [FEEDBACK_USED]</violation>

<violation number="2" location="tracecat/authz/seeding.py:448">
P2: Move the Organization import to the module top level to comply with the project rule that imports must not be inside function bodies.</violation>
</file>

<file name="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py">

<violation number="1" location="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py:25">
P1: Adding a NOT NULL column without a default value will fail if the table contains existing rows. Consider adding a `server_default` that references an existing role ID, or split this into multiple steps: add as nullable, populate data, then alter to NOT NULL.</violation>

<violation number="2" location="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py:40">
P1: Same issue as `invitation` table: adding a NOT NULL column without a default will fail if `organization_invitation` contains existing rows.</violation>
</file>

<file name="tracecat/authz/service.py">

<violation number="1" location="tracecat/authz/service.py:96">
P2: Use `result.tuples().all()` instead of `result.all()` for multi-column query results.

(Based on your team's feedback about using tuples().all() for multi-column queries.) [FEEDBACK_USED]</violation>
</file>

<file name="tracecat/workspaces/router.py">

<violation number="1" location="tracecat/workspaces/router.py:257">
P2: Membership lookup ignores org-wide role assignments (workspace_id=NULL), so a user with an org-wide role is returned as VIEWER. Include org-wide assignments when resolving the role.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@jordan-umusu jordan-umusu marked this pull request as ready for review February 2, 2026 19:56
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

membership = OrganizationMembership(
user_id=user_id,
organization_id=invitation.organization_id,
role=invitation.role,

P1 Badge Stop passing removed role field on org invite accept

The OrganizationMembership model no longer has a role column, but accept_invitation_for_user still instantiates it with role=invitation.role. On the first org invitation acceptance (the /invitations/accept flow used by the UI), SQLAlchemy will raise TypeError: 'role' is an invalid keyword argument, so users can’t accept invites at all. This should mirror the updated org-accept path by omitting role and creating a UserRoleAssignment from invitation.role_id instead.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 26 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py">

<violation number="1" location="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py:118">
P1: Downgrade will fail if tables contain data: `organization_invitation` and `invitation` add NOT NULL `role` columns without `server_default`. PostgreSQL cannot add a NOT NULL column to a non-empty table without a default value. Add `server_default` like the membership tables have.</violation>
</file>

<file name="tracecat/authz/service.py">

<violation number="1" location="tracecat/authz/service.py:99">
P1: Missing organization filter on org-wide role assignments could leak roles across organizations. When fetching org-wide assignments (`workspace_id IS NULL`), the query should also filter by `organization_id` to ensure data isolation between organizations.</violation>
</file>

<file name="tracecat/workspaces/router.py">

<violation number="1" location="tracecat/workspaces/router.py:385">
P2: Join should target RoleModel, not UserRoleAssignment, to avoid a self-join and ensure correct role lookup.</violation>

<violation number="2" location="tracecat/workspaces/router.py:399">
P2: Skip `role_slug is None` in get_workspace_membership so custom roles don’t block the org-wide fallback, matching list_memberships behavior.</violation>
</file>

<file name="tracecat/authz/seeding.py">

<violation number="1" location="tracecat/authz/seeding.py:279">
P2: seed_registry_scope is not concurrency-safe: it does a check-then-insert without ON CONFLICT handling, so concurrent seeding for the same action can raise a unique-constraint error. Use an upsert (ON CONFLICT DO NOTHING) or catch IntegrityError and re-select the existing scope to keep seeding idempotent.</violation>

<violation number="2" location="tracecat/authz/seeding.py:395">
P2: Role seeding is not concurrency-safe: the existence check + insert can race across processes and raise IntegrityError, breaking startup seeding. Use an upsert (ON CONFLICT DO NOTHING) or catch IntegrityError and re-query to keep seeding idempotent under concurrent startup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py">

<violation number="1" location="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py:135">
P2: Downgrade backfills every organization role as MEMBER, which discards real role assignments. Consider adding the column nullable, updating it from role_id via role.slug, then setting NOT NULL (and only add a default if the original schema had one).</violation>

<violation number="2" location="alembic/versions/4bb7e59026f3_drop_role_from_membership_and_.py:168">
P2: Downgrade backfills every workspace role as EDITOR, losing actual role assignments. Consider adding the column nullable, updating it from role_id via role.slug, then setting NOT NULL (and only add a default if the original schema had one).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from d1c99e7 to f0f545f Compare February 2, 2026 21:57
@jordan-umusu jordan-umusu changed the title feat(rbac): seed registry scopes during sync feat(rbac): Seed registry scopes during sync Feb 2, 2026
@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from f0f545f to 1b379d9 Compare February 2, 2026 22:34
@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from 1b379d9 to 7fc9fa9 Compare February 2, 2026 22:57
@jordan-umusu jordan-umusu force-pushed the feat/rbac-auth-layer branch 2 times, most recently from 87388ed to fe44660 Compare February 4, 2026 14:42
Copy link
Contributor

@daryllimyt daryllimyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase onto main

@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from 39c23c9 to 5dcdedf Compare February 9, 2026 15:14
Base automatically changed from feat/rbac-auth-layer to main February 9, 2026 15:43
@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from 5dcdedf to a34c7ca Compare February 9, 2026 16:54
@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from 7d82686 to ed1dadb Compare February 9, 2026 21:05
@blacksmith-sh

This comment has been minimized.

@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch 3 times, most recently from 7c8c52e to a76a788 Compare February 9, 2026 22:35
@jordan-umusu jordan-umusu force-pushed the feat/rbac-scope-seeding branch from a76a788 to c4ba9cc Compare February 10, 2026 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants